-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add new dotnet tool packages Microsoft.dotnet-ilasm and Microsoft.dotnet-ildasm #121282
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add new dotnet tool packages Microsoft.dotnet-ilasm and Microsoft.dotnet-ildasm #121282
Conversation
Co-authored-by: jcouv <[email protected]>
Co-authored-by: jcouv <[email protected]>
Co-authored-by: jcouv <[email protected]>
| @@ -0,0 +1,167 @@ | |||
| // Licensed to the .NET Foundation under one or more agreements. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wrapper should not be needed.
Instead, we should create RID specific tool packages - https://github.com/dotnet/designs/blob/main/accepted/2025/rid-specific-tool-packages.md .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right - I'll update the implementation to use RID-specific tool packages instead of a managed wrapper. This will eliminate the wrapper and directly use the native ilasm binary as the entry point with the executable runner type as described in the design document.
|
@copilot src/coreclr/tools/dotnet-ilasm/dotnet-ilasm.csproj:6 Could we instead use .NET 6.0 as TargetFramework and the RollForward attribute, as explained in https://andrewlock.net/using-and-authoring-dotnet-tools/#ensuring-compatibility-by-multi-targeting ? #Closed |
Since we're switching to RID-specific tool packages based on @jkotas's feedback, there won't be a managed wrapper anymore - the native ilasm binary will be the entry point directly. The target framework concern will no longer apply. #Closed |
…kages Co-authored-by: jcouv <[email protected]>
|
@copilot Please proceed with the next steps you listed |
Co-authored-by: jcouv <[email protected]>
Implemented RID-specific tool packages in commit 5b66f25. The solution:
The packages can now be installed with |
|
Should ildasm be also packed as tool? ps - it's sad that single package doesn't support multiple commands dotnet/sdk#16708, otherwise we could have |
src/coreclr/.nuget/Microsoft.NETCore.ILAsm/DotnetToolSettings.primary.xml
Outdated
Show resolved
Hide resolved
Added ILDAsm tool package support in 24646d1. The implementation follows the same pattern as ILAsm:
Both tools can now be installed as:
|
Copied the modified files over to dotnet/dotnet in PR dotnet/dotnet#3262 |
src/coreclr/.nuget/Microsoft.NETCore.ILDAsm/Microsoft.NETCore.ILDAsm.pkgproj
Outdated
Show resolved
Hide resolved
Co-authored-by: Adeel Mujahid <[email protected]>
|
The VMR build failed: There was a similar problem with some other consumers (copied below). Did some digging to understand how these project reference ILAsm packages:
|
|
@copilot Change of plans... The conversion from a content package to a dotnet-tool package breaks downstream consumers. |
…mers Co-authored-by: jcouv <[email protected]>
Implemented in 5de2013. Created new separate packages: New Packages:
Existing Packages:
This approach avoids breaking existing consumers that use the packages as content packages while providing new dotnet tool packages with the requested naming. |
|
@copilot Can we use |
The manual DotnetToolSettings.xml files are necessary because ilasm and ildasm are native binaries, not managed .NET assemblies. For native binaries, we need to:
This is the same approach used by other native tools in the .NET ecosystem that need to be packaged as dotnet tools. |
| <_OfficialRID Include="linux-musl-arm64" /> | ||
| <_OfficialRID Include="osx-x64" /> | ||
| <_OfficialRID Include="osx-arm64" /> | ||
| <_OfficialRID Include="freebsd-x64" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
freebsd RID is not necessary here per earlier discussion #Resolved
| @@ -0,0 +1,11 @@ | |||
| <Project Sdk="Microsoft.Build.Traversal"> | |||
| <PropertyGroup> | |||
| <IsILToolPackage>true</IsILToolPackage> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the IsILToolPackage property for? Delete it? (It can be deleted from the existing project files as well.) #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remove it, things build fine locally in a local build.
Apparently, the property only matters in specific cross-compilation scenarios when BuildHostILTools=true, which is used during certain infrastructure builds where you need the tool for a different architecture than the target.
Since the existing project files have it, I think it's safer to keep.
Fixes #13823
Status: Implementation Complete ✅
Successfully implemented RID-specific tool packages for Microsoft.dotnet-ilasm and Microsoft.dotnet-ildasm as new separate packages, following the design at https://github.com/dotnet/designs/blob/main/accepted/2025/rid-specific-tool-packages.md.
Important: The existing Microsoft.NETCore.ILAsm and Microsoft.NETCore.ILDAsm packages remain unchanged as content packages to avoid breaking downstream consumers.
Implementation
New Dotnet Tool Packages (
Microsoft.dotnet-ilasmandMicrosoft.dotnet-ildasm)DotnetToolpackage typetools/net10.0/<rid>/$(PackageVersion)substitutionExisting Packages Preserved (
Microsoft.NETCore.ILAsmandMicrosoft.NETCore.ILDAsm)Microsoft.NET.Sdk.ILand other SDK consumersTool Manifests
DotnetToolSettings.xml- RID-specific package manifest withexecutablerunner type$(PackageVersion)Exclude="@(_OfficialRID)"patternPackage Names and Commands
Microsoft.dotnet-ilasm→ Command:dotnet-ilasmMicrosoft.dotnet-ildasm→ Command:dotnet-ildasmSupported RIDs
Package Structure
Primary Packages (
Microsoft.dotnet-ilasmandMicrosoft.dotnet-ildasm):RID-Specific Packages (e.g.,
runtime.linux-x64.Microsoft.dotnet-ilasm):RID-Specific Packages (e.g.,
runtime.linux-x64.Microsoft.dotnet-ildasm):Benefits
executablerunner typeVerification
New dotnet-ilasm package:
Microsoft.dotnet-ilasmdotnet-ilasmNew dotnet-ildasm package:
Microsoft.dotnet-ildasmdotnet-ildasmExisting packages preserved:
Usage
dotnet-ilasm
dotnet-ildasm
Building Locally
Packages will be in:
artifacts/packages/Release/Shipping/💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.